Allow use of multiple extension galleries #2620
Conversation
oxy
force-pushed
the
multiple-galleries
branch
from
d025f0e to
e7a8d89
Compare
|
@oxy You should be able to fix the |
Just to clarify here - are only the first page of OpenVSX's extensions available with this change? Or are all of them available? |
| private api(path = ''): string { | ||
| return `${this.extensionsGalleryUrl}${path}`; | ||
| private api(path = ''): string[] { | ||
| if (!!this.extensionsGalleryUrl) { |
There was a problem hiding this comment.
Is the double ! to convert it to a boolean? Does it have a performance advantage over the Boolean function? Only asking/suggesting if it might improve readability - obviously your call :D
| if (!!this.extensionsGalleryUrl) { | |
| if (Boolean(this.extensionsGalleryUrl)) { |
There was a problem hiding this comment.
Perhaps I should just use this.isEnabled() instead - does the same check in practice but is probably ideal since the code already exists.
| } | ||
|
|
||
| async reportStatistic(publisher: string, name: string, version: string, type: StatisticType): Promise<void> { | ||
| // TODO: investigate further - currently we just send stats everywhere |
There was a problem hiding this comment.
I know folks have GitLens and tools to know who left a TODO comment but at least this puts your name right there so we don't forget.
| // TODO: investigate further - currently we just send stats everywhere | |
| // TODO(oxy): investigate further - currently we just send stats everywhere |
| } | ||
|
|
||
| async reportStatistic(publisher: string, name: string, version: string, type: StatisticType): Promise<void> { | ||
| // TODO: investigate further - currently we just send stats everywhere | ||
| // this is only used in one place - uninstall tracking - but |
There was a problem hiding this comment.
Unfinished comment? Was there more to this sentence?
There was a problem hiding this comment.
Oops, there was!
was supposed to say but unsure if MS will start using this elsewhere
This PR should implement support for using multiple extension galleries by default.
There's quite a few things that may be slightly hacky about it, since VSCode's extension gallery design is confusingly broken:
galleryUrlis used for/extensionqueryand uninstall statisticsitemUrlis supposedly used for linking to item pages but is not essential for functionality (and our gallery doesn't support it) so I just left it outcontrolUrlappears to be used for reporting malicious extensions, neither OpenVSX nor our gallery supports thisrecommendationsUrlprovides recommendations but neither OpenVSX nor our gallery supports thisAwkward thing here is that
galleryUrlon first glance appears to be the base URL for the whole API, and internally/extensionqueryis appended to it, but then they go ahead and include separate URLs for other extension gallery functionality - which is quite confusing.So, I just changed the type of
galleryUrlfromstringtostring[]and patched the methods that read from it.It might also be possible to change the type of
extensionGalleryto a list of objects instead of an object with a list ofgalleryUrl- but given that the other variables aren't really used by either of the two galleries we plan to primarily support (ours and OpenVSX's), so I tried to minimize engineering time there.I've tested the result by installing extensions from both OpenVSX and our repo - and it works fine!
Also, right now, I don't actually resort results to merge the two lists; it's just all of OpenVSX's first page, with all of Coder's (with duplicates filtered) after that.